Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two small improvements for named indexes U-Z #385

Merged
merged 6 commits into from
Oct 20, 2024
Merged

Conversation

muzimuzhi
Copy link
Collaborator

Currently,

  • Named indexes U-Z are supported in both from- and to-part of simple selectors like cell{-}{U-Z}={preto=x}, but only in to-part of child selectors odd/even. Thus cell{-}{even[2-Z]}={preto=x} is supported but cell{-}{even[X-Z]}={preto=x} is not.
  • The indexes converted from U-Z may be non-positive (hence invalid for following processes) in tables with small number of columns or rows.

This PR adds support for from-part of odd/even selectors and ensures the converted indexes are at least 1.

@muzimuzhi muzimuzhi changed the title Two small fixes for named indexes U-Z Two small improvements for named indexes U-Z Mar 18, 2023
@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

I think it will bring more and more troubles if a package starts to correct wrong user inputs: to me X is a wrong input if a table has only two rows and should cause errors.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

@muzimuzhi Could you help to confirm that l3build doesn't normalize end-of-line characters in .tlg files? It seems the eol characters are different on macos/linux.

@muzimuzhi
Copy link
Collaborator Author

muzimuzhi commented Mar 18, 2023

I think it will bring more and more troubles if a package starts to correct wrong user inputs: to me X is a wrong input if a table has only two rows and should cause errors.

It depends on how intelligent the language/package want to be. Named indexes U-Z are already part of that intelligence, and child selectors a family of another. For example python's slicing supports

>>> a = [1,2,3]
>>> a[-10:-1]
[1, 2]
>>> a[-10:10]
[1, 2, 3]

Currently

\documentclass{article}
\usepackage{tabularray}

\begin{document}
% `cell{-}{Y-Z}` works
\begin{tblr}{
  hlines, vlines,
  cell{-}{1-X}={preto=x}
}
  & & & \\
\end{tblr}

% `cell{-}{U-Z}` explodes
\begin{tblr}{
  hlines, vlines,
  % U = -6, Z = -1, four-column table
  cell{-}{U-X}={preto=x}
}
  & & & \\
\end{tblr}
\end{document}

throws low level error and the non-positive index is treated as zero. (In either way a fallback index is used.)

! Missing number, treated as zero.
<to be read again> 
                   ;
l.20 \end
         {tblr}

Thus I suggest to give a more helpful error message and (still) use 1 as fallback value.

Update: Then this seems to introduce a new message named index-out-of-bound which should be applied to several places with appropriate fallbacks. I don't want to introduce a big modification in current PR so if you don't like it, I can remove the resizing changes and leave the support for named indexes in from-part of odd/even.

@muzimuzhi
Copy link
Collaborator Author

muzimuzhi commented Mar 18, 2023

Could you help to confirm that l3build doesn't normalize end-of-line characters in .tlg files? It seems the eol characters are different on macos/linux.

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

Just guessing, git configs core.eol and core.autocrlf may help.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

See extra-002.tlg file in this pull request. It is quite annoying that the diff of this file is wrong.

@muzimuzhi
Copy link
Collaborator Author

l3build won't normalize EOL. The .tlg is generated on macOS and checked by the GitHub Actions workflow on Ubuntu and Windows. What's the concrete problem/difference you noticed?

See extra-002.tlg file in this pull request. It is quite annoying that the diff of this file is wrong.

It's OS specific. Windows uses CRLF while *-nix tends to use LF, see https://www.wikiwand.com/en/Newline#Representation. As I appended to #385 (comment),

Just guessing, git configs core.eol and core.autocrlf may help.

and GitHub also provides a setting to hide whitespace when reviewing.

I will try with git configs and perhaps push a commit to use CRLF.

@lvjr
Copy link
Owner

lvjr commented Mar 18, 2023

Just guessing, git configs core.eol and core.autocrlf may help.

I always turn off this autocrlf option in my local git repos, because I believe git should not touch these kind of things. It is a job of repo maintainers and build tools to decide and normalize eol characters.

But l3build is missing this feature. Maybe someday I will add some code to build.lua to normalize eol characters (always using LF characters) of tlg files. It seems doable.

@davidcarlisle
Copy link

you could open an issue at L3build

@muzimuzhi
Copy link
Collaborator Author

l3build won't normalize EOL.

I was wrong by using wrong patterns %r/%n yesterday. An issue is opened at l3build, latex3/l3build#293.

@muzimuzhi
Copy link
Collaborator Author

Just guessing, git configs core.eol and core.autocrlf may help.

I always turn off this autocrlf option in my local git repos, because I believe git should not touch these kind of things. It is a job of repo maintainers and build tools to decide and normalize eol characters.

Seems it's not always like this, for example front-end tools babel and npm both don't normalize EOL, see latex3/l3build#293 (comment).

EOL are special characters that are (almost) transparent to human and most modern programs that read in files, and perhaps only meaningful for softwares that compute file checksums or do file comparisons. So it's not too bad for git to touch EOLs.

@lvjr lvjr merged commit 6c65d61 into lvjr:main Oct 20, 2024
@lvjr lvjr added this to the 2025A milestone Oct 20, 2024
@lvjr
Copy link
Owner

lvjr commented Nov 25, 2024

Revisiting this eol issue, I think it makes sense to use .gitattributes file. I have added it and updated all .tlg files. (I was worrying that l3build would fail if .tlg and .log files had different eol characters. Now I know this is not true.)

@muzimuzhi
Copy link
Collaborator Author

l3build doesn't normalize EOL when saving, but does normalize EOLs when checking. As a proof, the CTeX-org/ctex-kit run tests on all three OS, using the same set of .tlg (example passed run).

Either I didn't know the full story or I expressed it ambiguously when composing #385 (comment).

@muzimuzhi muzimuzhi deleted the named-index branch November 25, 2024 14:06
@lvjr
Copy link
Owner

lvjr commented Nov 29, 2024

@muzimuzhi With *.tlg text eol=lf eol characters in hvline-004.tlg are different from those in other tlg files in my Windows computer. Do you think it is a good idea to change it to *.tlg text eol=auto? I guess it will not affect files in GitHub repository?

@muzimuzhi
Copy link
Collaborator Author

I have no idea...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants